The current PR cannot be reviewed against its stated purpose. Its title and body describe version tags and a diff_versions tool, but the branch currently contains the same body-token extractor change as PR #1060 and no versioning or MCP-tool implementation. Please restore the intended branch contents or close this PR if it was superseded; we should not review or merge a title/diff mismatch.

You were right, the branch did contain the body-token change from #1060 instead of the versioning work. That was a mistake in how I built the branch, not a stale description. It has since been rebuilt from scratch off main with the intended contents, and I've just rebased it again onto current main to clear the merge conflict.

What's in the diff now:

• version_tag plumbed through the pipeline (cbm_pipeline_set_version(), stamped onto nodes at insert time, including the ObjectScript parallel extraction path)
• diff_versions MCP tool: added/removed node detection between two version tags via OPTIONAL MATCH anti-joins
• optional version argument on index_repository, plus derivation from a trailing version-like path segment (/repos/MyApp/28.0 -> 28.0)
• diff_versions registered in TOOL_ANNOTATIONS as read-only, non-destructive, idempotent, and in the annotation-coverage test

Incremental indexing is skipped when a version tag is set, since a version stamp needs a full pass to be coherent.

Sorry for the wasted review cycle on the mismatch. Ready for review when you get to it, and no rush, I know #1063 is also in your queue.

Reviewed in depth. Thank you for the care in how you brought this — and I owe you an apology up front: you asked "is there appetite for this feature?" in #1062 and never got an answer before you built it. That silence is on us, and it is the reason you spent effort on something that now needs a direction decision it should have had first.

Contributor hygiene here was model. Issue first, clean DCO'd commits, the parallel extraction path threaded correctly via extract_ctx_t.version_tag in pass_parallel.c (easy to miss, and people usually do), and honest tool annotations — read-only, idempotent, non-destructive — mirrored in the coverage test.

Two findings you will want regardless of which way the direction call goes, because both mean the feature cannot work today.

1. The diff queries cannot parse. They use a variable property reference as a map value:

{name: n.name, version: '…'}

Our Cypher subset does not support that. parse_props() in src/cypher/cypher.c does expect(p, TOK_STRING) — property maps accept string literals only, so n.name lexes as IDENT DOT IDENT and the parse fails. Both queries error on every invocation.

2. Two versions can never coexist in one store. Setting a version forces the full-index path, and that path runs try_incremental_or_delete_db() in src/pipeline/pipeline.c, which logs "deleting old db" and cbm_unlinks the database before the full pass. So indexing 30.0 erases 28.0. Using different project names does not help either — those are separate DB files and resolve_store(project) opens exactly one. The "both versions accumulate side-by-side" premise is contradicted by the code the PR modifies. (Even within a single run, cbm_gbuf_upsert_node dedups by qualified name.)

Why this shipped looking fine: handle_diff_versions ignores both cbm_cypher_execute return codes, so a parse failure is indistinguishable from "nothing changed" and the tool confidently returns {"added":[],"removed":[]}. A single functional test calling the tool against a seeded store would have caught both defects immediately — right now the only test line registers annotations, which passes whether or not the tool does anything.

One thing I would ask you to drop in any revision, independent of everything else. cbm_derive_version_from_path treats any repository whose basename is digits and dots as versioned — /builds/20260731, …/1.2.3, …/2024.1. For those users, incremental indexing is silently disabled forever: every index becomes a database delete plus a full reindex, which at our scale is minutes and tens of gigabytes of RSS, plus version-stamped props they never asked for. Version should stay an explicit opt-in parameter.

On cost, since it matters for the direction call: the tag is a per-node string copy rather than an interned id — ,"version":"28.0" is 17 bytes per node, roughly 145 MB at 8.5M nodes, paid in graph-buffer RAM, in the store, and in the persistence artifact. No schema change and no forced migration, though, so legacy indexes are unaffected — that part is clean.

What is actually being decided. The interesting question is not the tool; it is whether we want a multi-version snapshot concept in the graph data model at all. With N versions co-resident, search_graph and trace_path would return N nodes per symbol, which is direct findability pollution and cuts against our first priority. For what it is worth, diff_versions is not a duplicate of detect_changes — different axis, indexed-snapshot comparison versus git-diff blast radius — but it is a canned two-query wrapper over the same Cypher engine query_graph already exposes, so the tool is the cheap part. If the answer is yes, the shape is probably store-level snapshots, or per-version projects with a cross-store diff, rather than a property on every node.

That is with the maintainer now, along with the two defects above so the decision is made on accurate information. I will come back to you with a real answer this time.

Two smaller notes for whenever it resumes: the diff identity uses short name rather than qualified_name, so same-named symbols across packages collapse and renames double-count; and the tool description promises "changed" nodes while only added and removed exist.